Fix Devportal application details layout shift when custom leftMenu width is applied#1277
Fix Devportal application details layout shift when custom leftMenu width is applied#1277
Conversation
…idth is applied and correct mobile alignment
rvrv1234
requested review from
Krishanx92,
lasanthaS,
piyumaldk and
tharikaGitHub
as code owners
|
rvrv1234 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
WalkthroughTwo component files receive maintenance updates: one simplifies asynchronous API call handling with early validation and consolidated promise chains, while the other refines layout margin calculations for responsive navigation styling without altering core functionality. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
portals/devportal/src/main/webapp/source/src/app/components/Applications/Details/index.jsx (1)
447-458:⚠️ Potential issue | 🔴 CriticalBox component still uses hardcoded margin subtractions inconsistent with the fixed
classes.content.The
Boxcomponent at lines 451-456 contains hardcoded values that were removed fromclasses.content:
- Line 451:
theme.custom.leftMenu.width - 158- Lines 455-456:
theme.custom.leftMenu.width - 4Meanwhile,
classes.content(line 137) correctly usestheme.custom.leftMenu.widthwithout subtractions and sets responsivemarginLeftto0. The Box component should align with this pattern by applyingclassName={classes.content}instead of using inlinesxwith hardcoded values, as was done in commit d19b8bf before being reverted.🐛 Suggested fix
- <Box sx={(theme) => ({ - display: 'flex', - flex: 1, - flexDirection: 'column', - marginLeft: (theme.custom.leftMenu.position === 'vertical-left' ? (theme.custom.leftMenu.width - 158) : 0), - paddingBottom: theme.spacing(3), - overflowX: 'hidden', - [theme.breakpoints.down('md')]: { - marginLeft: (theme.custom.leftMenu.position === 'vertical-left' - ? (theme.custom.leftMenu.width - 4) : 0) !== 0 && 50, - }, - })} - > + <Box className={classes.content}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@portals/devportal/src/main/webapp/source/src/app/components/Applications/Details/index.jsx` around lines 447 - 458, Replace the inline sx usage on the Box component with the existing classes.content so it uses theme.custom.leftMenu.width (no hardcoded subtractions) and the same responsive marginLeft behavior as classes.content; locate the Box instance that currently sets marginLeft via (theme.custom.leftMenu.width - 158) and the breakpoint block using (theme.custom.leftMenu.width - 4) and remove those calculations, applying className={classes.content} instead, preserving any non-margin styles (paddingBottom, overflowX, display/flex settings) by moving them into the stylesheet for classes.content or combining them with the className.
🧹 Nitpick comments (1)
portals/devportal/src/main/webapp/source/src/app/components/Applications/Details/index.jsx (1)
137-144: Fix looks correct, but indentation is inconsistent.The removal of the hardcoded
-4subtraction and setting margins to zero at themdbreakpoint aligns with the PR objectives. However, the indentation inside the breakpoint block (lines 142-144) is inconsistent with the rest of the file's style (compare with lines 94-96).🔧 Fix indentation to match file conventions
[theme.breakpoints.down('md')]: { - marginLeft: 0, - marginRight: 0, -}, + marginLeft: 0, + marginRight: 0, + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@portals/devportal/src/main/webapp/source/src/app/components/Applications/Details/index.jsx` around lines 137 - 144, The breakpoint style block for theme.breakpoints.down('md') has inconsistent indentation; locate the style object that contains marginLeft, marginRight, paddingBottom, overflowX (the same object using theme.custom.leftMenu) and reformat the lines inside the [theme.breakpoints.down('md')] block so their indentation matches the file's convention (align child properties with the other style blocks e.g., the same number of spaces/tabs used at lines like 94-96), ensuring marginLeft: 0 and marginRight: 0 remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@portals/admin/src/main/webapp/source/src/app/components/APISettings/UsageReport.jsx`:
- Around line 253-256: In the fetchTransactionData Promise chain (the .catch()
attached after the fetch that logs "Error fetching transaction count:"), remove
the rethrow (throw error) so the error is only logged and not rethrown; keep the
console.error call and allow the existing .finally() to run and
setLoading(false), since callers of fetchTransactionData are not awaiting its
promise and rethrowing causes unhandled promise rejections.
---
Outside diff comments:
In
`@portals/devportal/src/main/webapp/source/src/app/components/Applications/Details/index.jsx`:
- Around line 447-458: Replace the inline sx usage on the Box component with the
existing classes.content so it uses theme.custom.leftMenu.width (no hardcoded
subtractions) and the same responsive marginLeft behavior as classes.content;
locate the Box instance that currently sets marginLeft via
(theme.custom.leftMenu.width - 158) and the breakpoint block using
(theme.custom.leftMenu.width - 4) and remove those calculations, applying
className={classes.content} instead, preserving any non-margin styles
(paddingBottom, overflowX, display/flex settings) by moving them into the
stylesheet for classes.content or combining them with the className.
---
Nitpick comments:
In
`@portals/devportal/src/main/webapp/source/src/app/components/Applications/Details/index.jsx`:
- Around line 137-144: The breakpoint style block for
theme.breakpoints.down('md') has inconsistent indentation; locate the style
object that contains marginLeft, marginRight, paddingBottom, overflowX (the same
object using theme.custom.leftMenu) and reformat the lines inside the
[theme.breakpoints.down('md')] block so their indentation matches the file's
convention (align child properties with the other style blocks e.g., the same
number of spaces/tabs used at lines like 94-96), ensuring marginLeft: 0 and
marginRight: 0 remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac1caac9-ff60-4b58-8e32-13df3a162447
📒 Files selected for processing (2)
portals/admin/src/main/webapp/source/src/app/components/APISettings/UsageReport.jsxportals/devportal/src/main/webapp/source/src/app/components/Applications/Details/index.jsx
| .catch((error) => { | ||
| console.error('Error fetching transaction count:', error); | ||
| throw error; | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "UsageReport.jsx" -type fRepository: wso2/apim-apps
Length of output: 143
🏁 Script executed:
wc -l ./portals/admin/src/main/webapp/source/src/app/components/APISettings/UsageReport.jsxRepository: wso2/apim-apps
Length of output: 147
🏁 Script executed:
cat -n ./portals/admin/src/main/webapp/source/src/app/components/APISettings/UsageReport.jsxRepository: wso2/apim-apps
Length of output: 14911
Remove the throw statement from the .catch() block to prevent unhandled promise rejection.
The error at line 255 is already logged, and since fetchTransactionData doesn't return the promise and call sites don't await it (lines 264, 329), rethrowing creates an unhandled promise rejection. The .finally() block will still execute to set setLoading(false).
Suggested fix
.catch((error) => {
console.error('Error fetching transaction count:', error);
- throw error;
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@portals/admin/src/main/webapp/source/src/app/components/APISettings/UsageReport.jsx`
around lines 253 - 256, In the fetchTransactionData Promise chain (the .catch()
attached after the fetch that logs "Error fetching transaction count:"), remove
the rethrow (throw error) so the error is only logged and not rethrown; keep the
console.error call and allow the existing .finally() to run and
setLoading(false), since callers of fetchTransactionData are not awaiting its
promise and rethrowing causes unhandled promise rejections.
2 participants
Description
This PR fixes layout issues in the Devportal Application Details page.
Issue
Two UI problems were observed:
theme.custom.leftMenu.widthis customized.Root Cause
The layout margin used
(theme.custom.leftMenu.width - 4), which caused incorrect alignment when custom widths were applied.Additionally, the mobile breakpoint forced a fixed margin that resulted in a shifted layout.
Fix
Test
Summary by CodeRabbit